-
-
Notifications
You must be signed in to change notification settings - Fork 436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ended the request (and close the session) before core_app_run_after event #1592
Conversation
…mpleted rendering. Add core_app_run_after to allow events to occur after response is sent. Refs OpenMage#113
the core_app_run_after event won't be able to access any session based data right? |
I've not tried this.. I think the $_SESSION variable is still present with the same values but I'm guessing that modifying it would simply have no effect. |
In general, this makes me uneasy. I would offload anything that I wanted to run after to a queue. |
its described in this comment here: https://www.php.net/manual/en/function.session-write-close.php#112681
So, are we sure there is noone who still access the session after this here was called (in index.php)? also its probably better to check for the session status instead of the variable https://www.php.net/manual/en/function.session-status.php |
I tested this PR to sync data after each orders.
Not sure if there is another solution? I also getting a new problem. We are generating our product images with multi threads. With this PR, it continue to work, but, we are waiting all threads to end in a I can fix my problem with a new event before |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In production for > 4 months.
We must also update EVENTS.md.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated EVENTS file
} else { | ||
flush(); | ||
} | ||
if (isset($_SESSION)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we use https://www.php.net/manual/en/function.session-status.php here as @Flyingmana suggested?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description (*)
This PR does two things:
core_app_run_after
event to allow events to occur after response is sent.With this PR you can add an event observer for
core_app_run_after
which can do something which will not add any time to the response even if it is long-running (e.g.sleep(3);
).Related Pull Requests
Closes #113
Manual testing scenarios (*)
Questions or comments
Contribution checklist (*)